Skip to content

sp: prevent converting from string to num if config is set#7005

Merged
edsiper merged 3 commits intofluent:masterfrom
nokute78:fix_5629
Sep 23, 2023
Merged

sp: prevent converting from string to num if config is set#7005
edsiper merged 3 commits intofluent:masterfrom
nokute78:fix_5629

Conversation

@nokute78
Copy link
Contributor

@nokute78 nokute78 commented Mar 12, 2023

This patch is to fix #2498 #5629
TODO: add test code.

This patch is to support following configuration.

Key Description Default
sp.convert_from_str_to_num If enabled, Stream processor converts from number string to number type. true

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • [N/A] Backport to latest stable release.

Configuration

a.conf:

[SERVICE]
    Streams_File stream.conf
    Parsers_File parsers.conf
    sp.convert_from_str_to_num false

[INPUT]
    Name tail
    Path b.log
    Parser json
    Read_From_Head true

[OUTPUT]
    Name stdout
    Match test

stream.conf:

[STREAM_TASK]
    Name sample
    Exec CREATE STREAM test WITH (tag='test') AS SELECT word, num, COUNT(*) FROM STREAM:tail.0 WINDOW TUMBLING (1 SECOND) GROUP BY word, num;

b.log:

{"date": "22/abr/2019:12:43:51 -0600", "ip": "73.113.230.135", "word": "balsamine", "country": "Japan", "flag": false, "num": 96}
{"date": "22/abr/2019:12:43:52 -0600", "ip": "242.212.128.227", "word": "23456", "country": "Chile", "flag": false, "num": 15}
{"date": "22/abr/2019:12:43:52 -0600", "ip": "85.61.182.212", "word": "elicits", "country": "Argentina", "flag": true, "num": 73}

Debug/Valgrind output

Following error will be fixed by #7004

$ valgrind --leak-check=full bin/fluent-bit -c ~/git/WORKTREE/fix_5629/build/a.conf 
==47513== Memcheck, a memory error detector
==47513== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==47513== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==47513== Command: bin/fluent-bit -c /home/taka/git/WORKTREE/fix_5629/build/a.conf
==47513== 
Fluent Bit v2.1.0
* Copyright (C) 2015-2022 The Fluent Bit Authors
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

[2023/03/12 14:18:40] [ info] [fluent bit] version=2.1.0, commit=e209745f5e, pid=47513
[2023/03/12 14:18:40] [ info] [storage] ver=1.4.0, type=memory, sync=normal, checksum=off, max_chunks_up=128
[2023/03/12 14:18:40] [ info] [cmetrics] version=0.5.8
[2023/03/12 14:18:40] [ info] [ctraces ] version=0.3.0
[2023/03/12 14:18:40] [ info] [input:tail:tail.0] initializing
[2023/03/12 14:18:40] [ info] [input:tail:tail.0] storage_strategy='memory' (memory only)
[2023/03/12 14:18:40] [ info] [output:stdout:stdout.0] worker #0 started
[2023/03/12 14:18:40] [ info] [input:stream_processor:test] initializing
[2023/03/12 14:18:40] [ info] [input:stream_processor:test] storage_strategy=(null)
[2023/03/12 14:18:40] [ info] [sp] stream processor started
[2023/03/12 14:18:40] [ info] [sp] registered task: sample
[2023/03/12 14:18:40] [ info] [input:tail:tail.0] inotify_fs_add(): inode=4209349 watch_fd=1 name=b.log
[0] test: [1678598379.944006594, {""=>"balsamine", ""=>96, "COUNT(*)"=>1}]
[1] test: [1678598379.956341834, {""=>"23456", ""=>15, "COUNT(*)"=>1}]
[2] test: [1678598379.956385237, {""=>"elicits", ""=>73, "COUNT(*)"=>1}]
^C[2023/03/12 14:19:42] [engine] caught signal (SIGINT)
[2023/03/12 14:19:42] [ warn] [engine] service will shutdown in max 5 seconds
[2023/03/12 14:19:42] [ info] [input] pausing tail.0
[2023/03/12 14:19:42] [ info] [input] pausing test
[2023/03/12 14:19:42] [ info] [engine] service has stopped (0 pending tasks)
[2023/03/12 14:19:42] [ info] [input] pausing tail.0
[2023/03/12 14:19:42] [ info] [input] pausing test
[2023/03/12 14:19:42] [ info] [input:tail:tail.0] inotify_fs_remove(): inode=4209349 watch_fd=1
[2023/03/12 14:19:43] [ info] [output:stdout:stdout.0] thread worker #0 stopping...
[2023/03/12 14:19:43] [ info] [output:stdout:stdout.0] thread worker #0 stopped
==47513== 
==47513== HEAP SUMMARY:
==47513==     in use at exit: 172 bytes in 2 blocks
==47513==   total heap usage: 4,981 allocs, 4,979 frees, 3,419,150 bytes allocated
==47513== 
==47513== 23 bytes in 1 blocks are definitely lost in loss record 1 of 2
==47513==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==47513==    by 0x1B9657: flb_malloc (flb_mem.h:80)
==47513==    by 0x1B983A: sds_alloc (flb_sds.c:41)
==47513==    by 0x1B98C1: flb_sds_create_len (flb_sds.c:62)
==47513==    by 0x1B9967: flb_sds_create (flb_sds.c:88)
==47513==    by 0x242C87: flb_cf_section_property_get_string (flb_config_format.c:301)
==47513==    by 0xA5C272: sp_config_file (flb_sp.c:110)
==47513==    by 0xA5D787: flb_sp_create (flb_sp.c:704)
==47513==    by 0x2028C2: flb_engine_start (flb_engine.c:802)
==47513==    by 0x1A1B1F: flb_lib_worker (flb_lib.c:629)
==47513==    by 0x4FD7B42: start_thread (pthread_create.c:442)
==47513==    by 0x5068BB3: clone (clone.S:100)
==47513== 
==47513== 149 bytes in 1 blocks are definitely lost in loss record 2 of 2
==47513==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==47513==    by 0x1B9657: flb_malloc (flb_mem.h:80)
==47513==    by 0x1B983A: sds_alloc (flb_sds.c:41)
==47513==    by 0x1B98C1: flb_sds_create_len (flb_sds.c:62)
==47513==    by 0x1B9967: flb_sds_create (flb_sds.c:88)
==47513==    by 0x242C87: flb_cf_section_property_get_string (flb_config_format.c:301)
==47513==    by 0xA5C2E7: sp_config_file (flb_sp.c:117)
==47513==    by 0xA5D787: flb_sp_create (flb_sp.c:704)
==47513==    by 0x2028C2: flb_engine_start (flb_engine.c:802)
==47513==    by 0x1A1B1F: flb_lib_worker (flb_lib.c:629)
==47513==    by 0x4FD7B42: start_thread (pthread_create.c:442)
==47513==    by 0x5068BB3: clone (clone.S:100)
==47513== 
==47513== LEAK SUMMARY:
==47513==    definitely lost: 172 bytes in 2 blocks
==47513==    indirectly lost: 0 bytes in 0 blocks
==47513==      possibly lost: 0 bytes in 0 blocks
==47513==    still reachable: 0 bytes in 0 blocks
==47513==         suppressed: 0 bytes in 0 blocks
==47513== 
==47513== For lists of detected and suppressed errors, rerun with: -s
==47513== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

nokute78 added 2 commits March 5, 2023 16:11
@nokute78
config: support sp.convert_from_str_to_num
50c9085 
Signed-off-by: Takahiro Yamashita <nokute78@gmail.com>
@nokute78
sp: prevent converting from string to num if config is set
e209745 
Signed-off-by: Takahiro Yamashita <nokute78@gmail.com>
@nokute78 nokute78 marked this pull request as draft March 12, 2023 05:20
@nokute78 nokute78 temporarily deployed to pr March 12, 2023 05:21 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr March 12, 2023 05:21 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr March 12, 2023 05:21 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr March 12, 2023 05:43 — with GitHub Actions Inactive
@nokute78
tests: internal: sp: add test case for conv_from_str_to_num
5f9d20b 
Signed-off-by: Takahiro Yamashita <nokute78@gmail.com>
@nokute78
Copy link
Contributor Author

nokute78 commented Apr 16, 2023

I added a test code and this PR is ready to review.

@nokute78 nokute78 marked this pull request as ready for review April 16, 2023 00:57
@nokute78 nokute78 temporarily deployed to pr April 16, 2023 00:57 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr April 16, 2023 00:57 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr April 16, 2023 00:58 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr April 16, 2023 01:17 — with GitHub Actions Inactive
@koleini
Copy link
Contributor

koleini commented May 9, 2023 •
edited
Loading

Curious to know what is the behavior of other stream processors in such cases (strings that may represent a number).

@nokute78
Copy link
Contributor Author

nokute78 commented May 11, 2023

I'm not familiar with other stream processors.
In my opinion, it would be better to temporarily convert the type to check condition, but eventually revert to the initial type.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 10, 2023

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Aug 10, 2023
@edsiper edsiper merged commit 8f408d9 into fluent:master Sep 23, 2023
@nokute78 nokute78 deleted the fix_5629 branch September 24, 2023 00:54
This was referenced Sep 24, 2023
Closed
Merged
@nokute78
Copy link
Contributor Author

nokute78 commented Sep 24, 2023

Doc PR is fluent/fluent-bit-docs#1204

@Porkepix Porkepix mentioned this pull request Sep 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@koleini koleini Awaiting requested review from koleini koleini is a code owner

@edsiper edsiper Awaiting requested review from edsiper edsiper is a code owner

@leonardo-albertovich leonardo-albertovich Awaiting requested review from leonardo-albertovich

@fujimotos fujimotos Awaiting requested review from fujimotos

Assignees

No one assigned

Labels

docs-required Stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Stream Processing converts strings to numbers

3 participants

@nokute78 @koleini @edsiper